refactor(dev/docker/hive): shrink hive Docker image size by 420MB #3268

unknowntpo · 2024-05-05T02:14:04Z

What changes were proposed in this pull request?

Use multi-stage build to reduce hive image size from 2.27GB to 1.83GB.

I use first stage to download archive file and unzip them, then in the final stage, copied them to destination directory.

Why are the changes needed?

Fix: #3262

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Unit tests.

TODO

@mchades needs to push new image release to docker hub
@unknowntpo should update changelog

unknowntpo · 2024-05-05T05:34:28Z

reopen for rerunning ci tests

unknowntpo · 2024-05-05T09:58:46Z

@mchades Would you like to review this PR ?

unknowntpo · 2024-05-05T10:00:26Z

...common/src/test/java/com/datastrato/gravitino/integration/test/container/ContainerSuite.java

@@ -83,6 +83,7 @@ public void startHiveContainer() {
          HiveContainer.Builder hiveBuilder =
              HiveContainer.builder()
                  .withHostName("gravitino-ci-hive")
+                  .withImage("unknowntpo/gravitino-ci-hive:latest")


I use my hive image here, we need to upload newly created gravitino-ci-hive image before merging.

Hi, @unknowntpo , It's not good practice to use your image here, @xunliu Please help to verify if changes about the Hive image are reasonable, If everything is OK, we can release the image before this PR is merged, then @unknowntpo can change the image to the newly released one.

@unknowntpo Another point is that you may need to modify the related document about the Hive image, please see
https://github.com/datastrato/gravitino/blob/14297cddc894a4d47851ddfbcaec50bc547e0387/docs/docker-image-details.md?plain=1#L87C1-L88C1

ok, if @xunliu find this modification reasonable and push new image to docker hub, I'll update the changelog in docs.

I've add some todos in the PR description.

Hi, @unknowntpo , It's not good practice to use your image here, @xunliu Please help to verify if changes about the Hive image are reasonable, If everything is OK, we can release the image before this PR is merged, then @unknowntpo can change the image to the newly released one.

@unknowntpo Another point is that you may need to modify the related document about the Hive image, please see https://github.com/datastrato/gravitino/blob/14297cddc894a4d47851ddfbcaec50bc547e0387/docs/docker-image-details.md?plain=1#L87C1-L88C1

@yuqi1129 Can you help release this Docker image to hub?

Hi, @unknowntpo , It's not good practice to use your image here, @xunliu Please help to verify if changes about the Hive image are reasonable, If everything is OK, we can release the image before this PR is merged, then @unknowntpo can change the image to the newly released one.
@unknowntpo Another point is that you may need to modify the related document about the Hive image, please see 14297cd/docs/docker-image-details.md?plain=1#L87C1-L88C1

@yuqi1129 Can you help release this Docker image to hub?

OK

zhoukangcn · 2024-05-08T06:00:34Z

dev/docker/hive/Dockerfile

@@ -45,8 +64,6 @@ RUN apt-get update && apt-get upgrade -y && apt-get install --fix-missing -yq \
 RUN mkdir /root/.ssh
 RUN cat /dev/zero | ssh-keygen -q -N "" > /dev/null && cat /root/.ssh/id_rsa.pub > /root/.ssh/authorized_keys

-COPY packages /tmp/packages


you can use ADD, if use ADD in docker file for a zip file, docker builder will copy and unzip it, it will reduce the image size.

then, seems we do not need two stage build.

Thanks for you advise! I used ADD in the beginning, but I found that we need --strip-component to remove outer directory, ADD cannot do this.

Yes, --strip-component did not worked in ADD, so I use a soft link in Doris Image

Okay, I'll try the soft-link approach, thanks 😀

xunliu · 2024-05-08T09:50:40Z

dev/docker/hive/Dockerfile

@@ -3,14 +3,33 @@
 # This software is licensed under the Apache License version 2.
 #

-FROM ubuntu:16.04
-LABEL maintainer="support@datastrato.com"


I think we need kept LABEL maintainer="support@datastrato.com" in the here.

Since I don't use 2 stage build right now, this review is outdated.

xunliu · 2024-05-08T09:53:19Z

dev/docker/hive/Dockerfile

@@ -3,14 +3,33 @@
 # This software is licensed under the Apache License version 2.
 #

-FROM ubuntu:16.04
-LABEL maintainer="support@datastrato.com"
+FROM ubuntu:16.04 AS packages


I think better change to FROM ubuntu:16.04 AS install-hadoop, The packages name ease confuse with /tmp/packages directory.

Since I don't use 2 stage build right now, this is not needed.

xunliu · 2024-05-09T02:00:01Z

dev/docker/hive/Dockerfile

+
+# hadoop
+RUN mkdir ${HADOOP_HOME}
+RUN tar -xz -C ${HADOOP_HOME} --strip-components 1 -f /tmp/packages/${HADOOP_PACKAGE_NAME} && rm -rf /tmp/packages/${HADOOP_PACKAGE_NAME}


Why we need use --strip-component params?

same reason as #3268 (comment)

unknowntpo · 2024-05-12T05:33:25Z

I use soft-link approach as @zhoukangcn mentioned at #3268 (comment), because we don't need to maintain lots of env var like JAVA_HOME=/usr/local/jdk at 1st stage and final stage, which is a little bit ugly.
c.c. @xunliu , @mchades

xunliu · 2024-05-17T06:12:29Z

hi @unknowntpo thank you for your continue work.
Please attach your latest Docker image size screen snapshot in the comments.
Use soft link also shrink size by 500MB?

unknowntpo · 2024-05-17T06:38:34Z

@xunliu Shrinked hive Docker image: 1.85 GB (with soft-link)

Original Docker image: 2.27 GB

It's not 500 MB, but 420 MB 😭

xunliu · 2024-05-21T13:58:27Z

hi @unknowntpo Please fix branch conflicts. Thanks

xunliu

LGTM

yuqi1129 · 2024-05-22T03:51:41Z

@unknowntpo
Could you also modify the change log?
please see: https://github.com/datastrato/gravitino/blob/5a4931b1ce6e94e32003cffa17c8e955518039dd/docs/docker-image-details.md?plain=1#L103-L105

yuqi1129 · 2024-05-22T03:58:57Z

I have released gravitino-ci-hive:0.1.12. Please upgrade it to Gravitino from version 0.1.11 to 0.1.12.

unknowntpo · 2024-05-22T05:50:51Z

I have released gravitino-ci-hive:0.1.12. Please upgrade it to Gravitino from version 0.1.11 to 0.1.12.

@yuqi1129 Done.

yuqi1129 · 2024-05-22T05:54:47Z

I have released gravitino-ci-hive:0.1.12. Please upgrade it to Gravitino from version 0.1.11 to 0.1.12.

@yuqi1129 Done.

I mean you need to change the following place:

There should be several similar places.

yuqi1129

LGTM

unknowntpo · 2024-05-22T06:50:30Z

@yuqi1129 done, the tests has passed.

@mchades

) ### What changes were proposed in this pull request? Use multi-stage build to reduce hive image size from 2.27GB to 1.83GB. I use first stage to download archive file and unzip them, then in the final stage, copied them to destination directory. ### Why are the changes needed? Fix: #3262 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unit tests. ## TODO - [ ] @mchades needs to push new image release to docker hub - [ ] @unknowntpo should update changelog

@mchades

…ache#3268) ### What changes were proposed in this pull request? Use multi-stage build to reduce hive image size from 2.27GB to 1.83GB. I use first stage to download archive file and unzip them, then in the final stage, copied them to destination directory. ### Why are the changes needed? Fix: apache#3262 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unit tests. ## TODO - [ ] @mchades needs to push new image release to docker hub - [ ] @unknowntpo should update changelog

unknowntpo changed the title ~~refactor(dev/docker/hive): shrink hive Docker image size by 200MB~~ refactor(dev/docker/hive): shrink hive Docker image size by 500MB May 5, 2024

unknowntpo closed this May 5, 2024

unknowntpo reopened this May 5, 2024

unknowntpo marked this pull request as ready for review May 5, 2024 09:58

unknowntpo commented May 5, 2024

View reviewed changes

mchades requested review from xunliu and mchades May 6, 2024 12:27

yuqi1129 assigned unknowntpo May 6, 2024

unknowntpo force-pushed the feat-shrink-docker-image-size branch from aadf3c6 to b4fd3a5 Compare May 8, 2024 03:51

zhoukangcn reviewed May 8, 2024

View reviewed changes

xunliu reviewed May 9, 2024

View reviewed changes

unknowntpo force-pushed the feat-shrink-docker-image-size branch from b4fd3a5 to 8c3ffa5 Compare May 12, 2024 03:37

unknowntpo changed the title ~~refactor(dev/docker/hive): shrink hive Docker image size by 500MB~~ refactor(dev/docker/hive): shrink hive Docker image size by 420MB May 17, 2024

unknowntpo force-pushed the feat-shrink-docker-image-size branch from 4fc6112 to 38fde2d Compare May 21, 2024 14:29

xunliu previously approved these changes May 22, 2024

View reviewed changes

unknowntpo dismissed xunliu’s stale review via 6b8a3e1 May 22, 2024 04:33

unknowntpo force-pushed the feat-shrink-docker-image-size branch from 6b8a3e1 to 87acb60 Compare May 22, 2024 04:35

unknowntpo added 5 commits May 22, 2024 13:01

refactor(dev/docker/hive): shrink hive Docker image size by 200MB

cb42b79

feat(hive): reduce hive container size from 2.27GB to 1.76GB

3c74f90

feat(hive): clean up

87230d6

feat(hive): remove unused debug log

193921d

feat(hive): use latest tag

52b5939

unknowntpo added 4 commits May 22, 2024 13:01

feat(hive:Dockerfile): rollback to old Dockerfile

332004c

feat(hive): use ln

daad34f

feat(hive): minor tweaks

47a4149

feat(hive): minor tweaks

4c75621

unknowntpo force-pushed the feat-shrink-docker-image-size branch from 87acb60 to 200c0e8 Compare May 22, 2024 05:03

change hive image back to gravitino-ci-hive, update changelog

acf79aa

unknowntpo force-pushed the feat-shrink-docker-image-size branch from 200c0e8 to acf79aa Compare May 22, 2024 06:01

yuqi1129 approved these changes May 22, 2024

View reviewed changes

yuqi1129 added the branch-0.5 label May 22, 2024

yuqi1129 merged commit 506963e into apache:main May 22, 2024
22 checks passed

unknowntpo deleted the feat-shrink-docker-image-size branch May 22, 2024 07:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(dev/docker/hive): shrink hive Docker image size by 420MB #3268

refactor(dev/docker/hive): shrink hive Docker image size by 420MB #3268

unknowntpo commented May 5, 2024 •

edited

Loading

unknowntpo commented May 5, 2024

unknowntpo commented May 5, 2024

unknowntpo May 5, 2024

yuqi1129 May 6, 2024

unknowntpo May 8, 2024

unknowntpo May 8, 2024

xunliu May 22, 2024

yuqi1129 May 22, 2024

zhoukangcn May 8, 2024

unknowntpo May 8, 2024 •

edited

Loading

zhoukangcn May 8, 2024

unknowntpo May 8, 2024

xunliu May 8, 2024

unknowntpo May 12, 2024

xunliu May 8, 2024

unknowntpo May 12, 2024

xunliu May 9, 2024

unknowntpo May 12, 2024

unknowntpo commented May 12, 2024 •

edited

Loading

xunliu commented May 17, 2024

unknowntpo commented May 17, 2024

xunliu commented May 21, 2024

xunliu left a comment

yuqi1129 commented May 22, 2024 •

edited

Loading

yuqi1129 commented May 22, 2024

unknowntpo commented May 22, 2024

yuqi1129 commented May 22, 2024

yuqi1129 left a comment

unknowntpo commented May 22, 2024

refactor(dev/docker/hive): shrink hive Docker image size by 420MB #3268

refactor(dev/docker/hive): shrink hive Docker image size by 420MB #3268

Conversation

unknowntpo commented May 5, 2024 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

TODO

unknowntpo commented May 5, 2024

unknowntpo commented May 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unknowntpo May 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unknowntpo commented May 12, 2024 • edited Loading

xunliu commented May 17, 2024

unknowntpo commented May 17, 2024

xunliu commented May 21, 2024

xunliu left a comment

Choose a reason for hiding this comment

yuqi1129 commented May 22, 2024 • edited Loading

yuqi1129 commented May 22, 2024

unknowntpo commented May 22, 2024

yuqi1129 commented May 22, 2024

yuqi1129 left a comment

Choose a reason for hiding this comment

unknowntpo commented May 22, 2024

unknowntpo commented May 5, 2024 •

edited

Loading

unknowntpo May 8, 2024 •

edited

Loading

unknowntpo commented May 12, 2024 •

edited

Loading

yuqi1129 commented May 22, 2024 •

edited

Loading